"Fix" bug 22175: cdstreq should always use ES:DI for 16-size structs (DO NOT MERGE- but, please, help?)#13255
Conversation
|
Thanks for your pull request and interest in making D better, @FeepingCreature! We are looking forward to reviewing it, and you should be hearing from a maintainer soon.
Please see CONTRIBUTING.md for more information. If you have addressed all reviews or aren't sure how to proceed, don't hesitate to ping us with a simple comment. Bugzilla references
Testing this PR locallyIf you don't have a local development environment setup, you can use Digger to test this PR: dub run digger -- build "master + dmd#13255" |
FeepingCreature
changed the title
|
ping @WalterBright , could you give us some pointers? |
… or not we're on a platform with segmented access.
FeepingCreature
force-pushed
the
fix/bug-22175-experimental-backend-poking
branch
from
3a34ec2 to
38b3280
Compare
|
Okay, what's up here? Pushed a rebase... I'm still not really up with merging this, but it doesn't seem like anything else is going to happen here? I guess I'd rather have the commit in than not, if those are the only two options on the table? |
|
This is the PR that caused the regression: #12409 |
|
This fix doesn't work because it needs to do a dereference of DI. Replaced with #13788 |
4 participants
Do not merge! I definitely don't understand what I'm doing here! This is presented as a request for comments!
Okay.
Something very weird is going on with 22175. It needs to do a struct assignment from AX|DX, because it's trying to
?:merge a 9-byte (that is, rounded up to 16-byte) struct. But it falls into a codepath incdstreqwhere I think it thinks it's trying to work with pointers? So it does some sort of weird fixup and then decides to only use DI, not DI|ES, because we're on x86_64 and we don't need segment addressing (ES:DI) there. But we're working with astreqelem, and the result of that isn't a pointer to begin with?? So then we're stuck trying to fit a 16-byte result in a 8-byte register and crash.The PR just removes that EX_flat check, which gives it enough space to fit AX|DX into. This seems to pass the testsuite, so technically it's a fix, but the goal here is more to get some eyes on that codepath than to get this PR merged.
Help pls? 🥺